-
Notifications
You must be signed in to change notification settings - Fork 23
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement support for the PROXY protocol #108
base: main
Are you sure you want to change the base?
Conversation
internal/sshserver/serverImpl.go
Outdated
@@ -15,6 +15,7 @@ import ( | |||
messageCodes "github.com/containerssh/libcontainerssh/message" | |||
"github.com/containerssh/libcontainerssh/service" | |||
"golang.org/x/crypto/ssh" | |||
"github.com/pires/go-proxyproto" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a third party library and should be abstracted away with its own package, including tests so it can be replaced if need be.
internal/sshserver/serverImpl.go
Outdated
netListener, err := listenConfig.Listen(lifecycle.Context(), "tcp", s.cfg.Listen) | ||
if err != nil { | ||
s.lock.Unlock() | ||
return messageCodes.Wrap(err, messageCodes.ESSHStartFailed, "failed to start SSH server on %s", s.cfg.Listen) | ||
} | ||
if useProxy { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems to me that PROXY is a wrapper protocol like many others (e.g. websocket). We should consider creating a generic abstraction for wrapper protocols.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't foresee having any other wrapping protocols in-front of the ssh server since, we can consider that if it's necessary further down the line.
@janosdebugs This one is ready for another review as well |
Please describe the change you are making
Implement support for the PROXY protocol. Sadly this cannot easily be tested as there are no user-visible changes if this is implemented correctly. (Header is transparently accepted/rejected)
Are you the owner of the code you are sending in, or do you have permission of the owner?
Sent with permission of the owner
The code will be published under the MIT-0 license. Have you read and understood this license?
Yes